Recoil -> Jotai Conversion #467
Conversation
| @@ -429,33 +430,34 @@ export const GraphList: FC<{ onRunGraph?: (graphId: GraphId) => void }> = memo(( | |||
| toast.error('A graph or folder with that name already exists.'); | |||
| return; | |||
| } | |||
| setSavedGraphs( | |||
There was a problem hiding this comment.
Recoil supports updater functions automatically, Jotai apparently doesn't. Have to get the savedGraphs ahead of time then use them to do the update. Risk here is using stale data.
There was a problem hiding this comment.
Yeah I am worried about that, but as long as we test it to make sure there's nothing lost anywhere we're probably okay? A lot of it has to do with loading and saving graphs, and managing the "opened project" state
There was a problem hiding this comment.
I was incorrect regarding the jotai updater fn's. It is only an issue when you provide a custom setter function in the atom definition like this:
export const connectionsState = atom(
(get) => get(graphState).connections,
(get, set, newValue: NodeConnection[]) => {
const currentGraph = get(graphState);
set(graphState, { ...currentGraph, connections: newValue });
},
);
When you use that atom in a component and call a setter on it, the new value passed in is required to be of type NodeConnection[]. This is why I thought jotai had issues with updaters.
You can allow flexibility by allowing for the newValue to be either the new value or an updater fn, like so:
export const connectionsState = atom(
(get) => get(graphState).connections,
(get, set, newValue: NodeConnection[] | ((prev: NodeConnection[]) => NodeConnection[])) => {
const currentGraph = get(graphState);
const currentConnections = currentGraph.connections;
const nextConnections = typeof newValue === 'function' ? newValue(currentConnections) : newValue;
set(graphState, { ...currentGraph, connections: nextConnections });
}
);
|
I love this! Thanks! We'll need to address the local storage conflict problem first, but other than that, some testing and we should be good to go |
Recoil has been deprecated + it doesn't work with React v19. This PR replaces Recoil with Jotai.
Outstanding issues that need to be fixed:
React.setStateand Recoil do. Instead of using an updater, I retrieved the atom's existing data ahead of time and then used it during the update. The risk here is updating with stale data. See comment onGraphList.tsxfor example.Jotai's persist has per-atom keys. Net effect of this: app will crash on load if you have any persisted data due to the unexpected structure. App will load if you clear local storage.
To solve this, you can either add in a localstorage migrator that runs on startup, detects if localstorage has unexpected structure of data and either migrates/deletes it. Alternatively, can get jotai to store data in the same nested fashion. This is the easier fix and its what I'll likely do.